Skip to content

Conversation

@chrispader
Copy link
Member

@chrispader chrispader commented Feb 5, 2026

Make sure that the data from Nitro hybrid objects is directly passed to the user-facing JS API.


Note

Medium Risk
Changes the native-to-JS return path for execute/executeAsync by returning a shared HybridNitroSQLiteQueryResult instead of a copied JS-built object, which could affect result shape/lifetime and batch row counts.

Overview
Passes Nitro hybrid query results through to JS without rebuilding them. Native sqliteExecute() now returns a HybridNitroSQLiteQueryResult directly (removing SQLiteExecuteQueryResult), and HybridNitroSQLite::execute() just forwards that shared hybrid object.

HybridNitroSQLiteQueryResult now stores insertId, rowsAffected, results, rows, and metadata as its own fields and constructs the rows accessor up front (including an empty-rows case). JS execute/executeAsync stop constructing a new result object and instead return the hybrid object as-is, and TypeScript types are simplified to rely on the Nitro spec (only specializing rows generics).

Written by Cursor Bugbot for commit 078f30d. This will update automatically on new commits. Configure here.

@chrispader chrispader marked this pull request as ready for review February 11, 2026 09:31
@chrispader chrispader merged commit 5e108c5 into main Feb 11, 2026
3 of 5 checks passed
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 4 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on March 25

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

try {
const result = HybridNitroSQLite.execute(dbName, query, params)
return buildJsQueryResult<Row>(result)
return result as QueryResult<Row>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rows item contract changed unexpectedly

High Severity

Returning HybridNitroSQLite.execute*() directly exposes native rows behavior instead of the normalized JS shape from buildJsQueryResult. The rows.item callback now comes from native NitroSQLiteQueryResultRows and resolves asynchronously, so callers expecting synchronous rows.item(idx) can get a Promise instead of a row.

Additional Locations (1)

Fix in Cursor Fix in Web

"Bash(grep:*)"
]
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local Claude settings committed

Low Severity

The PR adds .claude/settings.local.json, a machine-local configuration file unrelated to the SQLite feature change. Committing local permissions.allow entries (Bash(find:*), Bash(grep:*)) can introduce environment-specific behavior and accidental policy drift in the repository.

Fix in Cursor Fix in Web

if (index >= rows.size()) {
return std::nullopt;
}
return rows[index];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Row index coercion changes item behavior

Medium Severity

rows.item now casts idx with static_cast<size_t>(idx), which coerces non-integer numbers to integer indices. The previous JS behavior (data[idx]) returned undefined for non-integer keys. This can return the wrong row for values like decimals or NaN, changing query result access semantics.

Fix in Cursor Fix in Web

export type QueryResult<Row extends QueryResultRow = QueryResultRow> =
NitroSQLiteQueryResult & {
readonly rowsAffected: number
readonly insertId?: number
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generic QueryResult typing lost for results

Low Severity

QueryResult<Row> no longer redefines results as Row[], so the generic type parameter is effectively ignored for main row data. execute<Row>() now returns broadly typed Record<string, SQLiteValue>[], reducing type safety and making typed query APIs less reliable for consumers.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant